Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #1041 #1049

Merged
merged 2 commits into from
Sep 14, 2022
Merged

fix #1041 #1049

merged 2 commits into from
Sep 14, 2022

Conversation

seeM
Copy link
Member

@seeM seeM commented Sep 13, 2022

@seeM seeM added the bug Something isn't working label Sep 13, 2022
@seeM seeM requested review from jph00 and hamelsmu September 13, 2022 11:42
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -79,7 +79,7 @@ def _clean_cell(cell, clear_all, allowed_metadata_keys, clean_ids):
# %% ../nbs/api/clean.ipynb 13
def clean_nb(
nb, # The notebook to clean
clear_all=False, # Remove all cell metadata and cell outputs
clear_all=False, # Remove all cell metadata and cell outputs?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think the question mark could be confusing? It is a bit confusing for me

Meaning I interpret it as we aren't sure what the program will do

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's something I noticed Jeremy doing to describe bool parameters and quite liked myself. I think you get used to reading the ? as not sounding confused 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha ok

@jph00 jph00 merged commit 52fb2ea into fastai:master Sep 14, 2022
@seeM seeM deleted the fix-clear-all branch September 14, 2022 01:36
@jph00
Copy link
Member

jph00 commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

setting clear_all=True in settings.ini is not respected
3 participants